Skip to content

fix(connpool): back off between failed dials in OneTimePool.Run - #849

Open
dwisiswant0 wants to merge 1 commit into
mainfrom
dwisiswant0/fix/connpool/back-off-between-failed-dials-in-onetimepool-run
Open

dwisiswant0 wants to merge 1 commit into
mainfrom
dwisiswant0/fix/connpool/back-off-between-failed-dials-in-onetimepool-run

Conversation

@dwisiswant0

Copy link
Copy Markdown
Member

Run discarded the dial error and redialed immediately, so a target that
refuses connections made the loop spin at tens of millions of dials per
second and pin a CPU core. Callers start Run in a goroutine and ignore
or only log its result (crypto/jarm, tlsx), so returning the first
error would instead leave every Acquire blocked until its own deadline
and turn one transient failure into a dead pool.

Retry failed dials with an exponential wait from 50ms up to 1s, reset by
the next successful dial. The wait selects on the pool context, so
Close ends it immediately.

The same loop had three related defects. The dialer was invoked while
holding the read lock, so Close blocked on the write lock for the whole
dial, and it was re-read after the lock was released and could be nil
by then. The dialer is now read once and called without the lock. The
default dial used net.Dial and ignored the pool context; it now uses
DialContext. Finally, a connection that finished dialing after Close had
swept the in-flight set was never closed; Run now closes a connection it
adds to a done pool.

Run now returns the context error wrapped as "connpool: run stopped:
..." instead of bare; the in-repo and known downstream callers ignore it
or match it with errors.Is.

Fixes: #839

Run discarded the dial error and redialed immediately, so a target that
refuses connections made the loop spin at tens of millions of dials per
second and pin a CPU core. Callers start Run in a goroutine and ignore
or only log its result (crypto/jarm, tlsx), so returning the first
error would instead leave every Acquire blocked until its own deadline
and turn one transient failure into a dead pool.

Retry failed dials with an exponential wait from 50ms up to 1s, reset by
the next successful dial. The wait selects on the pool context, so
Close ends it immediately.

The same loop had three related defects. The dialer was invoked while
holding the read lock, so Close blocked on the write lock for the whole
dial, and it was re-read after the lock was released and could be nil
by then. The dialer is now read once and called without the lock. The
default dial used net.Dial and ignored the pool context; it now uses
DialContext. Finally, a connection that finished dialing after Close had
swept the in-flight set was never closed; Run now closes a connection it
adds to a done pool.

Run now returns the context error wrapped as "connpool: run stopped:
..." instead of bare; the in-repo and known downstream callers ignore it
or match it with errors.Is.

Fixes: #839
Signed-off-by: Dwi Siswanto <git@dw1.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

connpool: OneTimePool.Run retries a failing dial tens of millions of times per second

1 participant